No actionable comments were generated in the recent review. 🎉

Walkthrough

在 ExternalTenantResourceTracker 中加入基于 TaskContext 的外部租户上下文回退与同步，更新消息发送/接收拦截器以读写 TaskContext；在 ExternalTenantZQLExtension 的子查询中可选加入按 userId 的额外过滤条件。

Changes

Sequence Diagram(s)

sequenceDiagram
    participant Sender as "消息发送方\n(线程)"
    participant TaskCtx as "TaskContext\n(任务范围存储)"
    participant MsgBus as "消息总线\n(拦截器)"
    participant Receiver as "消息接收方\n(线程 / Session)"
    participant DB as "持久层\n(ExternalTenant refs)"

    Sender->>TaskCtx: (可选) put TASK_CONTEXT_EXTERNAL_TENANT
    Sender->>MsgBus: sendMessage(msg)
    MsgBus->>TaskCtx: 若 ThreadLocal 缺失 -> 读取 TASK_CONTEXT_EXTERNAL_TENANT
    MsgBus->>MsgBus: 附加 'external-tenant-context' header（如存在）
    MsgBus->>Receiver: 投递消息
    Receiver->>Receiver: beforeDeliveryMessage: 读取 header 或 session
    Receiver->>TaskCtx: 写入或移除 TASK_CONTEXT_EXTERNAL_TENANT
    Receiver->>Receiver: 设置/清除 ThreadLocal ExternalTenantContext
    Receiver->>DB: notifyResourceOwnershipCreated（读取 TaskContext/ThreadLocal）

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 我是小兔跳代码，TaskContext 把线包，
线程断了不慌张，租户上下文会回收，
拦截器里写又读，ZQL 多了 userId，
小兔鼓掌说太妙，追踪稳当又牢靠！ 🥕

❌ Failed checks (1 error, 1 warning)

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Comment from yaohua.wu:

Review: MR !9588 — ZCF-1147

概述：本 MR 修复了两个问题：(1) 级联资源（Volume/NIC/CdRom）在 thdf.chainSubmit() 线程池切换后 ExternalTenantContext ThreadLocal 丢失，导致 ExternalTenantResourceRefVO 未写入；(2) ZQL 查询缺少用户级隔离，同一租户下不同用户可见彼此资源。修复方案利用 TaskContext 跨线程传播机制做 fallback，并在 ExternalTenantZQLExtension 中追加可选的 userId SQL 子句。

整体方案方向正确，TaskContext fallback 的思路与 ZStack 现有的 HasThreadContextAspect/SetThreadContextAspect 机制吻合。但有一个 安全问题 需要在合入前修复。

🔴 Critical — SkipThisRestrictExprException 导致非法 userId 时整个租户过滤被跳过

文件: identity/src/main/java/org/zstack/identity/ExternalTenantZQLExtension.java，新增行 77-78

if (!SAFE_TENANT_VALUE.matcher(userId).matches()) {
    throw new SkipThisRestrictExprException();
}

问题：当 userId 不符合 SAFE_TENANT_VALUE 白名单正则时，抛出 SkipThisRestrictExprException 会导致 整条 ExternalTenantZQLExtension 生成的限制表达式被跳过——即不仅跳过 userId 子句，连 source + tenantId 的过滤也一并消失。

安全风险：攻击者只需构造一个不匹配正则的 userId（如 "a]]; DROP TABLE--"），就能使 ZQL 查询在 无任何租户隔离 的情况下执行，实现跨租户数据泄露（权限提升）。

建议修复：

• 方案 A（推荐）：对非法 userId 直接抛出明确的校验异常（如 throw new OperationFailureException(argerr("invalid external tenant userId: %s", userId))），拒绝本次查询。
• 方案 B：忽略 userId 子句，但仍保留 source + tenantId 过滤——即把 userFilter 设为 "" 而非抛异常。这会退化到租户级隔离，但不会完全无过滤。

🟡 Warning — TaskContext.putTaskContextItem() 可能接收 null 值导致 NPE

文件: identity/src/main/java/org/zstack/identity/ExternalTenantResourceTracker.java，新增行 116-117

ExternalTenantContext ctx = session.getExternalTenantContext();
ExternalTenantContext.setCurrent(ctx);
TaskContext.putTaskContextItem(TASK_CONTEXT_EXTERNAL_TENANT, ctx);

问题：hasExternalTenant() 的实现是 externalTenantContext != null && externalTenantContext.getTenantId() != null，但 getExternalTenantContext() 直接返回字段引用。虽然逻辑上在 hasExternalTenant() == true 时不应为 null，但这两个调用非原子操作（同一 SessionInventory 可能被并发修改）。更关键的是：TaskContext 内部使用 ConcurrentHashMap，调用 put(key, null) 会直接抛出 NullPointerException。同一 MR 中 ExternalTenantZQLExtension 已经在 hasExternalTenant() 之后显式检查了 tenantCtx == null，说明作者也意识到了这种不一致。

建议修复：在写入 TaskContext 前加 null 守卫：

if (session != null && session.hasExternalTenant()) {
    ExternalTenantContext ctx = session.getExternalTenantContext();
    if (ctx != null) {
        ExternalTenantContext.setCurrent(ctx);
        TaskContext.putTaskContextItem(TASK_CONTEXT_EXTERNAL_TENANT, ctx);
    } else {
        ExternalTenantContext.clearCurrent();
        TaskContext.removeTaskContextItem(TASK_CONTEXT_EXTERNAL_TENANT);
    }
}

🟡 Warning — TaskContext fallback 逻辑重复，应抽取公共方法

文件: ExternalTenantResourceTracker.java，行 82-88 与行 152-158

以下 fallback 逻辑在 beforeSendMessage() 和 notifyResourceOwnershipCreated() 中完全重复：

if (ctx == null || ctx.getTenantId() == null) {
    Object taskCtx = TaskContext.getTaskContextItem(TASK_CONTEXT_EXTERNAL_TENANT);
    if (taskCtx instanceof ExternalTenantContext) {
        ctx = (ExternalTenantContext) taskCtx;
    }
}

建议抽取为 private static ExternalTenantContext resolveCurrentContext() 方法，减少维护成本，也避免未来新增调用点时遗漏 fallback。

🟢 Suggestion — 消息 header 中 userId 为 null 时的数组行为

文件: ExternalTenantResourceTracker.java，行 92

msg.putHeaderEntry(HEADER_EXTERNAL_TENANT,
    new String[]{ctx.getSource(), ctx.getTenantId(), ctx.getUserId()});

当 ctx.getUserId() 为 null 时，new String[]{..., null} 会在数组中放入 null 元素。虽然接收端 tenantData.length >= 3 ? tenantData[2] : null 能正确处理，但建议显式判断：仅当 userId != null 时才放入三元素数组，否则沿用旧的两元素格式，保持向后兼容性并避免序列化层面的意外行为。

🟢 Suggestion — 注释完善

TASK_CONTEXT_EXTERNAL_TENANT 常量上方的 Javadoc 注释写得很清晰，建议在 beforeSendMessage() 和 notifyResourceOwnershipCreated() 的 fallback 代码块注释中补充一句："此处不设置 ThreadLocal，因为当前线程的 ThreadLocal 状态应由 SetThreadContextAspect 在入口处恢复，此处仅读取"，帮助后续维护者理解为什么 fallback 只读不写。

总结

结论：🔴 Critical 问题是安全漏洞，建议修复后再合入。其余问题可视情况在后续迭代中处理。

🤖 Robot Reviewer